feat: expose stratum mining stats via Owner API#3897
Conversation
wiesche89
left a comment
There was a problem hiding this comment.
This is a nice and useful feature, and the endpoint is wired to the live StratumStats correctly. Before merging, I’d clarify whether the API should return only active workers, avoid masking a missing provider as disabled, and preserve the public node_apis signature.
| /// Whether the stratum server is enabled in config | ||
| pub is_enabled: bool, | ||
| /// Whether the stratum server is currently running | ||
| pub is_running: bool, |
There was a problem hiding this comment.
The source flag is set on startup but never cleared on listener failure or shutdown. Is “currently running” too strong for the API contract?
There was a problem hiding this comment.
A port conflict still leaves this as true after the listener exits. Could we set it after a successful bind, clear it on exit, and test the bind-failure case?
Add get_mining_status to the Owner JSON-RPC API so headless operators can query the same stratum/worker stats shown on the TUI mining tab. Wire live StratumStats into node_apis and add `grin client miningstatus`. Closes mimblewimble#3201
f406b85 to
60178eb
Compare
- Filter mining status worker_stats to currently connected workers (StratumStats retains disconnected workers for the node lifetime), and drop the now always-true is_connected field from WorkerInfo - Return an internal error from get_mining_status when no stats provider is wired, instead of masking it as disabled - Restore the public node_apis signature; add node_apis_with_mining_stats for provider wiring - Document is_running as set-once at startup - Trim get_mining_status doc comment to match neighboring methods
60178eb to
120e0cd
Compare
| blocks_found: stats.blocks_found, | ||
| network_hashrate: stats.network_hashrate, | ||
| minimum_share_difficulty: stats.minimum_share_difficulty, | ||
| // worker_stats retains every worker that ever connected; only expose |
There was a problem hiding this comment.
The response is bounded now, but each request still scans the full worker history under the stats lock. Could we keep a bounded active-worker view and derive num_workers from it too?
| /// Whether the stratum server is enabled in config | ||
| pub is_enabled: bool, | ||
| /// Whether the stratum server is currently running | ||
| pub is_running: bool, |
There was a problem hiding this comment.
A port conflict still leaves this as true after the listener exits. Could we set it after a successful bind, clear it on exit, and test the bind-failure case?
| pub worker_stats: Vec<WorkerInfo>, | ||
| } | ||
|
|
||
| impl Default for MiningStatus { |
There was a problem hiding this comment.
This has no caller besides its own test and duplicates StratumStats::default(). Could we remove the impl and test to avoid default drift?
There was a problem hiding this comment.
Removed the Default impl and its unit test so we do not duplicate / drift from StratumStats::default.
| } | ||
|
|
||
| #[test] | ||
| fn get_mining_status_with_provider() { |
There was a problem hiding this comment.
Could we test this once through the generated JSON RPC handler? The current tests only call Owner directly, so the wire response is not covered.
There was a problem hiding this comment.
Done — get_mining_status is now covered through the generated OwnerRpc JSON-RPC handler (success wire payload + missing-provider error path).
| pub fn get_mining_status(&self) -> Result<MiningStatus, Error> { | ||
| match &self.mining_stats { | ||
| Some(provider) => Ok(provider()), | ||
| None => Err(Error::Internal( |
There was a problem hiding this comment.
This is fixed now. Could you also update the PR description? It still says a missing provider returns a disabled default.
There was a problem hiding this comment.
PR description updated: missing provider returns an internal error; disabled stratum still returns is_enabled: false via the wired provider.
|
I’ve written a prototype Grin miner in Rust https://github.com/wiesche89/rust-miner . It can be used to test the new Owner API method end to end. I tested four connected miners this way. The API reported num_workers: 4 with four worker entries, then returned num_workers: 0 and an empty worker list after disconnecting them. C25 is only intended as a lightweight connection test, valid testnet mining requires C32. |
Remove the unused MiningStatus Default impl (and its test) to avoid drift from StratumStats::default. Cover get_mining_status through the generated OwnerRpc JSON-RPC handler so the wire response is tested.
Summary
Adds an Owner API endpoint for stratum mining statistics so operators running headless (no TUI) can inspect connected workers and share stats — the same data shown on the TUI mining tab.
Closes #3201
Changes
get_mining_statusmethod on Owner JSON-RPC (/v2/owner)MiningStatus/WorkerInfoin the API crateStratumStatsinto the Owner API via a provider callbackgrin client miningstatusdoc/api/api.mdExample
{ "jsonrpc": "2.0", "method": "get_mining_status", "params": [], "id": 1 }Returns whether stratum is enabled/running, worker count, block height, network difficulty/hashrate, blocks found, and per-worker share stats for currently connected workers. When stratum is disabled, a provider still returns
is_enabled: false. If no mining stats provider is wired into the Owner API,get_mining_statusreturns an internal error instead of masking the misconfiguration as disabled.Test plan
MiningStatusserde roundtripOwner::get_mining_statuswithout provider (error) and with provider via generated OwnerRpc JSON-RPC handler (wire response)StratumStats→MiningStatusconversion (filters disconnected workers)cargo check -p grin --bin gringrin client miningstatus